-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Gradual typing #1217
base: main
Are you sure you want to change the base?
Gradual typing #1217
Conversation
32341d4
to
7055239
Compare
@@ -35,7 +38,7 @@ class CoderPrompts: | |||
" stop and wait for your approval." | |||
) | |||
|
|||
repo_content_prefix = """Here are summaries of some files present in my git repository. | |||
repo_content_prefix: str | None = """Here are summaries of some files present in my git repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type union syntax X | Y
is only available in Python 3.10, but Aider still supports Python 3.9.
Instead, this should be
repo_content_prefix: str | None = """Here are summaries of some files present in my git repository. | |
repo_content_prefix: typing.Optional[str] = """Here are summaries of some files present in my git repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On line 1 of this file, I've added
from __future__ import annotations
which makes sure X | Y
is supported on Python 3.9.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to put that in the tests too it looks like:
TypeError: unsupported operand type(s) for |: 'type' and 'NoneType'
7055239
to
8058187
Compare
I would think that typing an untyped python codebase is exactly the kind of thing that Aider itself would be great at. Typing the edges is trivial, that leads to type errors, fix those, that leads to new type errors, fix those, etc. I also think if it's worth adding type annotations at all, then it's worth bumping to a Python version that supports the latest / greatest versions of them. Aider seems like more of an end-user thing than infrastructure library, seems okay for it to be more aggressive in bumping requirements than a typical library. |
Amazing work as this will make aider improving itself even better (from experience llms do quite well with typed code) |
0f49a3d
to
6416497
Compare
1a50afe
to
77438bb
Compare
I rebased on |
Have you considered pyright instead? Pyright is fast enough to work in your vscode and neovim live, and can go in your pre-commit. Also we should put typing into the github actions. Personally I think typing will make aider better both in scripting mode (making it more understandable to devs, less likely for bugs) and for self-modification via aider (it will improve the linting loop, and will give more informative repo maps). I'd definitely love to see a blog post about the accuracy of aider edits under properly typed vs untyped code generation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very minimal IMO. I like it (though I have no authority)
run: | | ||
python -m pip install --upgrade pip | ||
pip install '.[browser,dev,playwright,mypy]' | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add flake8 and black checks as they are in the pre-commit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to just add https://github.com/pre-commit/action
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few more lint comments
name: Linting | ||
|
||
on: | ||
push: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why we need this on push, just PR right?
- name: Set up Python 3.12 | ||
uses: actions/setup-python@v5 | ||
with: | ||
python-version: "3.12" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a matrix like in the other workflows
https://github.com/OnScale/aider-stateless/actions/runs/12914071198/job/36012781606?pr=7 |
- skip some directories - skip 3rd party packages without typing or type stubs - allow untyped globals and class attributes - don't check untyped functions and methods
Also avoid building a list and overwriting it with a tuple.
94c963c
to
c568cb0
Compare
Here's another stab at adding typing since #639 was closed.
I acknowledge that Paul has expressed that he isn't currently planning to add type hints, and that reviewing type hints would be a burden.
However, I believe this extremely minimal Mypy configuration and a minimal set of changes not only make Mypy pass, but also enable to use it to check some types, and allow development to continue without requiring full type hints everywhere.
Mypy takes over the burden of reviewing type annotations from humans.
Most notably, functions with no type hints are not checked by Mypy at all with this configuration. This allows adding type hints just only to select sections of the code base. It is still of course possible to gradually add typing overall and increase Mypy's strictness if desired.
See Using mypy with an existing codebase for more information.